Skip to content
This repository was archived by the owner on Aug 2, 2019. It is now read-only.

Conversation

@Natim
Copy link
Member

@Natim Natim commented Nov 12, 2014

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there any python lib that could handle this for us?
If not, I suggest you could at least move this piece of code to some utility function

@Natim
Copy link
Member Author

Natim commented Nov 12, 2014

Updated.

Makefile Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really see the point of this here, you could define it in your shell instead, no ?
(.phony is repeated, is that ok ?)

@leplatrem
Copy link
Contributor

I'm not against merging this, but a bit doubtful though.

We can have all this code in Daybed, but in the mean time, opening a PR on elasticsearch-py would be worth it IHMO. I don't see why it shouldn't support absolute urls for hosts!

@Natim
Copy link
Member Author

Natim commented Nov 12, 2014

Why should it?

@leplatrem
Copy link
Contributor

The dict formalism is rather curious, since there's a standard for that! RFC-1738 :)

@Natim
Copy link
Member Author

Natim commented Nov 13, 2014

I can make a pull-request to them based on this assumption. There is not urgency yet to merge this patch since I already applied it in production.

@Natim Natim force-pushed the elasticsearch_config branch from d28e89b to dcc5fab Compare November 13, 2014 09:29
@leplatrem
Copy link
Contributor

Ok.

Also, I wouldn't have created a dedicated module just for one utility function. But it may be a matter of taste :)
More generally, we should start thinking of #213 :)

@Natim
Copy link
Member Author

Natim commented Nov 13, 2014

More generally, we should start thinking of #213 :)

Exactly the reason why I did it like so :)

@Natim
Copy link
Member Author

Natim commented Nov 13, 2014

Here we go: elastic/elasticsearch-py#149

@leplatrem
Copy link
Contributor

Good job @Natim! Your PR was merged in ES python client :) We will soon be able to close this here!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants